feat: hydratable
#17154
feat: hydratable
#17154
Conversation
🦋 Changeset detectedLatest commit: bc9df88 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
| isomorphic_hydratable['get'] = get_hydratable_value; | ||
| isomorphic_hydratable['has'] = has_hydratable_value; | ||
| isomorphic_hydratable['set'] = () => e.fn_unavailable_on_client('hydratable.set'); |
There was a problem hiding this comment.
I'm not at all convinced we need these — can we get rid of them?
| return ` | ||
| <script> | ||
| { | ||
| const store = (window.__svelte ??= {}).h ??= new Map(); | ||
| for (const [k,v] of [${entries.join(',')}]) { | ||
| store.set(k, v); | ||
| } | ||
| } | ||
| </script>`; | ||
| } |
There was a problem hiding this comment.
as a purely aesthetic matter it would be nice if this was indented in a way that looks sensible when you view source
| return ` | |
| <script> | |
| { | |
| const store = (window.__svelte ??= {}).h ??= new Map(); | |
| for (const [k,v] of [${entries.join(',')}]) { | |
| store.set(k, v); | |
| } | |
| } | |
| </script>`; | |
| } | |
| return ` | |
| <script> | |
| { | |
| const store = (window.__svelte ??= {}).h ??= new Map(); | |
| for (const [k,v] of [${entries.join(',')}]) { | |
| store.set(k, v); | |
| } | |
| } | |
| </script>`; | |
| } |
| } | ||
| } | ||
|
|
||
| export class MemoizedUneval { |
There was a problem hiding this comment.
Do we really need this? Typically, each hydratable value would be different. And they will hydrate to different objects, even if they do in fact refer to the same object, because we're not unevaling everything at one (and can't, because of custom encode/decode logic). It seems like overhead
| * @template T | ||
| * @param {string} key | ||
| * @param {() => T} fn | ||
| * @param {{ transport?: Transport<T> }} [options] |
There was a problem hiding this comment.
I still think the third argument should probably be the transport options, unless we actually expect to add other options in future
| const store = get_render_context(); | ||
|
|
||
| if (store.hydratables.has(key)) { | ||
| e.hydratable_clobbering(key, store.hydratables.get(key)?.stack || 'unknown'); |
There was a problem hiding this comment.
I find myself wondering if we should just allow this — we can serialize the second value, and warn if there's a mismatch (i.e. the second serialization differs from the first) but otherwise just re-use the first value? If I have a (possibly library-provided) function that uses hydratable internally, it could be annoying to have to refactor my code to ensure that I only call it in one component rather than two.
Counter-argument: without also providing the ability to cache/dedupe, this could result in work (such as an API request/whatever) being done twice.
Before submitting the PR, please make sure you do the following
feat:,fix:,chore:, ordocs:.packages/svelte/src, add a changeset (npx changeset).Tests and linting
pnpm testand lint the project withpnpm lint